Skip to content

ARROW-6205: [C++] ARROW_DEPRECATED warning when including io/interfaces.h#5062

Closed
harrism wants to merge 3 commits intoapache:masterfrom
harrism:fix-deprecated-warning-nvcc
Closed

ARROW-6205: [C++] ARROW_DEPRECATED warning when including io/interfaces.h#5062
harrism wants to merge 3 commits intoapache:masterfrom
harrism:fix-deprecated-warning-nvcc

Conversation

@harrism
Copy link
Copy Markdown
Contributor

@harrism harrism commented Aug 12, 2019

ARROW_DEPRECATED on using statements causes warnings when including interfaces.h from CUDA source files (.cu) compiled with nvcc.

This PR works around this issue the same way it is done for MSVC by changing a single preprocessor condition.

Fixes JIRA 6205

Comment thread cpp/src/arrow/io/interfaces.h Outdated
@pitrou pitrou changed the title [ARROW-6205 ARROW_DEPRECATED warning when including io/interfaces.h from CUDA (.cu) source] Fix ARROW-6205: [C++] ARROW_DEPRECATED warning when including io/interfaces.h Aug 12, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 13, 2019

Codecov Report

Merging #5062 into master will increase coverage by 1.6%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5062      +/-   ##
==========================================
+ Coverage   87.61%   89.21%    +1.6%     
==========================================
  Files        1009      727     -282     
  Lines      144082   103189   -40893     
  Branches     1418        0    -1418     
==========================================
- Hits       126232    92057   -34175     
+ Misses      17488    11132    -6356     
+ Partials      362        0     -362
Impacted Files Coverage Δ
cpp/src/arrow/io/interfaces.h 90% <ø> (ø) ⬆️
cpp/src/arrow/json/converter.cc 90.05% <0%> (-1.76%) ⬇️
cpp/src/arrow/json/chunked-builder.cc 79.91% <0%> (-1.68%) ⬇️
python/pyarrow/tests/test_parquet.py 96.4% <0%> (-0.07%) ⬇️
go/arrow/ipc/writer.go
js/src/util/fn.ts
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
... and 277 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a3eab5...f4d89b3. Read the comment docs.

Copy link
Copy Markdown
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
We can merge this when CI is green and @harrism confirms that this change resolves the reported warning.

Copy link
Copy Markdown
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@wesm wesm closed this in e41ad0d Aug 13, 2019
@harrism
Copy link
Copy Markdown
Contributor Author

harrism commented Aug 13, 2019

@wesm did you intend to close this PR rather than merge it? it appears you agreed with @kou that it should be merged once CI passes.

@kou I can confirm that this change eliminates the warnings when building RAPIDS libcudf.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Aug 13, 2019

@harrism It was merged actually, we're just using a dedicated merge script rather than Github's PR merge button. So false alert there :-)

@wesm
Copy link
Copy Markdown
Member

wesm commented Aug 13, 2019

@harrism right, we don't use the GitHub UI to merge PRs, our merge tool creates atomic patches from each pull request and commits those to master, see

e41ad0d

I'm actually a bit surprised a project with many developers like RAPIDS is able to tolerate a "dirty" commit history (https://github.com/rapidsai/cudf/commits/branch-0.9). How do you expect to be able to git-bisect?

@harrism
Copy link
Copy Markdown
Contributor Author

harrism commented Aug 14, 2019

I've git bisected myself and so have others without problems. log(N) is a very powerful tool.

Thanks for accepting my patch.

@wesm
Copy link
Copy Markdown
Member

wesm commented Aug 14, 2019

The number of commits is not an issue -- it's a question of having a mix of "good" (green CI build) and "bad" (failing CI build) in the commit history. In any case, that's a discussion for another time and place :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants